-
Notifications
You must be signed in to change notification settings - Fork 15
Memory leaks fix, part I: create/destroy #155
Conversation
Frankly speaking, this entire code should not be necessary. If we really need all that dereferencing, then it means that something still retains the Also, like I said on the TM, I think that manually destroying UI components is also unnecessary. They should be destroyed by their parents or by the plugin which defined the given root UI component. I may be wrong, but I think we should look into this more closely because I think this is just hiding some lower-level issue that we have. |
Yes I they weren't destroyed by the parent Plugin - that's the part of the fix. I will review the code to what parts are really needed to fix "static" memory leaks (mostly the
|
I noticed certain instability when running tests. Locally:
note that a different editor type got memory bloated. |
ps.: It's only the builds which fails so it might be that some plugin which I didn't test in Other solution is to close an eye for builds and remove those tests ;) |
@oleq: after some debugging and also talking with @mlewand it seems that it might beneficial to add some timeouts after Could you test current state of branch constellation (sorry for messy commits - I needed to trigger CI couple of times) on you machine? Current configuration will run every test 10 times so it could take a while to complete. I'll remove unnecessary yarn test -cws --files=editor-*,build-* |
@oleq: It looks like the CI is stable enough. The only one fails is test timeout which might be due to "heavy" usage of |
LGTM |
a9da39c
to
2825fed
Compare
Suggested merge commit message (convention)
Fix: Fix memory leaks. Closes ckeditor/ckeditor5#1341.
Additional information
--expose-gc
to Chrome's flags in Karma launcher configuration. ckeditor5-dev#473.google-chrome \ -js-flags="--expose-gc" \ --enable-precise-memory-info \ --disable-extensions \ --disable-plugins \ --incognito \ http://localhost:8125
typing: PR(redundant)WIP - test are failing.